[client] Fix Exit Node submenu separator accumulation on Windows by tobsec · Pull Request #5691 · netbirdio/netbird

tobsec · 2026-03-25T12:28:42Z

Problem

On Windows, the tray app uses a 10-second background poller to refresh the Exit Node menu (because TrayOpenedCh is not supported on Windows). Every poll cycle that finds a selected exit node calls s.mExitNode.AddSeparator() before adding the "Deselect All" item.

AddSeparator() returns no handle, so the separator is never removed in the cleanup pass of recreateExitNodeMenu(). All other items — exit node checkboxes and "Deselect All" — are properly tracked and removed each cycle. The separator is not.

After the client has been running for a while with an exit node selected, hundreds of separator lines stack up in the submenu, filling the full screen height with blank entries.

On Linux/FreeBSD this doesn't manifest because the parent mExitNode item itself is removed and recreated each cycle (to work around a different systray limitation), which wipes all children including the orphaned separators.

Fix

Replace the untracked AddSeparator() call with a regular disabled sub-menu item. This gives us a handle that can be stored in mExitNodeSeparator and removed at the start of each recreateExitNodeMenu() call, alongside the existing mExitNodeDeselectAll cleanup.

// Before
s.mExitNode.AddSeparator()

// After
sep := s.mExitNode.AddSubMenuItem("───────────────", "")
sep.Disable()
s.mExitNodeSeparator = sep

The visual result is equivalent on Windows (a greyed-out, unclickable divider row), and the item is now properly cleaned up on every menu refresh.

Test

Run the Windows client with an exit node selected and leave it running for several minutes. Open the Exit Node submenu — previously it would grow with each poll; with this fix the menu stays clean regardless of uptime.

Documentation

Select exactly one:

I added/updated documentation for this change
Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Refactor
- Improved exit-node menu behavior: the separator now persists correctly across menu updates and the "Deselect All" option has been reorganized for more reliable presentation and interaction, reducing visual flicker and improving menu stability.

On Windows the tray uses a background poller (every 10s) instead of TrayOpenedCh to keep the Exit Node menu fresh. Each poll that has a selected exit node called s.mExitNode.AddSeparator() before the "Deselect All" item. Because AddSeparator() returns no handle the separator was never removed in the cleanup pass of recreateExitNodeMenu(), while every other item (exit node checkboxes and the "Deselect All" entry) was properly tracked and removed. After the client has been running for a while with an exit node selected this leaves hundreds of separator lines stacked in the submenu, filling the screen height with blank entries (netbirdio#4702). On Linux/FreeBSD this is masked because the parent mExitNode item itself is removed and recreated each cycle, wiping all children including orphaned separators. Fix: replace the untracked AddSeparator() call with a regular disabled sub-menu item that is stored in mExitNodeSeparator and removed at the start of each recreateExitNodeMenu() call alongside mExitNodeDeselectAll. Fixes netbirdio#4702

coderabbitai · 2026-03-25T12:28:57Z

📝 Walkthrough

Walkthrough

Adds persistent tracking for the exit-node separator menu item and refactors menu recreation to remove and re-add that separator via a new helper, preventing duplicate separators when rebuilding the exit-node submenu.

Changes

Cohort / File(s)	Summary
Exit node UI state & behavior `client/ui/client_ui.go`, `client/ui/network.go`	Added `mExitNodeSeparator` field to `serviceClient`; refactored `recreateExitNodeMenu` to remove the previous separator before rebuild and introduced `addExitNodeDeselectAll()` to create/store a disabled separator (`"───────────────"`) and the “Deselect All” item with its click goroutine.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

[client, gui] fix exit nodes menu on reconnect, remove tooltips #5167: Modifies exit-node menu handling in recreateExitNodeMenu (descriptor/tooltip handling and exitNode reset) — closely related to menu logic changes.
[client] Fix exit node menu not refreshing on Windows #5553: Changes exit-node menu/deselect-all and separator behavior — directly related to this PR's area.
[client] Update fyne and add exit menu retry #5187: Adds tray-open-triggered updateExitNodes and retry behavior tied to exit-node population — interacts with exit-node UI flows.

Suggested reviewers

braginini
lixmal

Poem

🐰 I hopped through menus, found lines in a row,
They multiplied each refresh — oh no!
I store one brave separator, keep duplicates at bay,
Hop, click, rebuild — now only one line will stay. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code changes directly address issue `#4702` by replacing untracked AddSeparator() with a managed disabled submenu item stored in mExitNodeSeparator.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the separator accumulation issue; the helper refactoring (addExitNodeDeselectAll) is directly related and reduces complexity.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description provides comprehensive context: it identifies the root cause (untracked AddSeparator call), explains platform-specific behavior, describes the fix with code examples, and includes testing instructions.
Title check	✅ Passed	The title accurately captures the main purpose of the PR: fixing a Windows-specific bug where Exit Node submenu separators accumulate over time.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Move the separator + deselect-all creation and its goroutine listener out of recreateExitNodeMenu into a dedicated helper, bringing the function's cognitive complexity back under the SonarCloud threshold.

sonarqubecloud · 2026-03-28T12:36:47Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mlsmaycon · 2026-03-30T08:41:48Z

Thanks for the contribution @tobsec

client/ui: extract addExitNodeDeselectAll to reduce cognitive complexity

8e00199

Move the separator + deselect-all creation and its goroutine listener out of recreateExitNodeMenu into a dedicated helper, bringing the function's cognitive complexity back under the SonarCloud threshold.

mlsmaycon changed the title ~~[client/ui] Fix Exit Node submenu separator accumulation on Windows~~ [client,ui] Fix Exit Node submenu separator accumulation on Windows Mar 29, 2026

mlsmaycon changed the title ~~[client,ui] Fix Exit Node submenu separator accumulation on Windows~~ [client] Fix Exit Node submenu separator accumulation on Windows Mar 29, 2026

mlsmaycon approved these changes Mar 30, 2026

View reviewed changes

mlsmaycon merged commit 13807f1 into netbirdio:main Mar 30, 2026
41 of 49 checks passed

lixmal mentioned this pull request Apr 8, 2026

Windows Client Context Menu buggy (multiple divider lines for "Exit Node" section) #5760

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[client] Fix Exit Node submenu separator accumulation on Windows#5691

[client] Fix Exit Node submenu separator accumulation on Windows#5691
mlsmaycon merged 2 commits intonetbirdio:mainfrom
tobsec:fix/exit-node-tray-separator-accumulation

tobsec commented Mar 25, 2026 •

edited by mlsmaycon

Loading

Uh oh!

coderabbitai Bot commented Mar 25, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

sonarqubecloud Bot commented Mar 28, 2026

Uh oh!

Uh oh!

mlsmaycon commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tobsec commented Mar 25, 2026 • edited by mlsmaycon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Test

Documentation

Docs PR URL (required if "docs added" is checked)

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

sonarqubecloud Bot commented Mar 28, 2026

Quality Gate passed

Uh oh!

Uh oh!

mlsmaycon commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tobsec commented Mar 25, 2026 •

edited by mlsmaycon

Loading

coderabbitai Bot commented Mar 25, 2026 •

edited

Loading